-
Notifications
You must be signed in to change notification settings - Fork 25
Improve README #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve README #57
Conversation
@mattytrentini perhaps you could take a look at the new README too. Since you suggested we could improve it, I would appreciate your feedback. Anything that's missing, or maybe too much? Anything that could be even clearer or friendlier to newcomers? |
Make it friendlier and easier to get started: * Generally shorten sentences/paragraphs, where possible * Add Installation and Getting Started instructions * Mention counter.py example as the simplest way to get started * Shorten supported/not-supported features into a bulleted list * Add Advanced Usage section showing how to assemble to and load from file * Add example of how to assemble on a PC * Add reference to GitHub Actions workflow for how to run tests * Add mention of ESP32-S2 not being supported * Add 2 more links to ULP examples * Add section mentioning the license * Remove reference to missing stuff, bugs and other beta quality symptoms * Remove mention of the issue tracker, as this is common on GitHub * Remove old README from examples dir
Differentiate between a main heading and subsection headings. This mirrors the change also done in the main README.
Links are relative to the current repo, so when forking the repo, the build status refers to the fork rather than the parent repo.
e96cf1b
to
7c4b153
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good changes overall, but it might be a bit much now for just taking "as is" for the pypi pkg (see setup.py reading in the README.rst).
usually i try to have the README as sorts of an "elevator speech" just shortly pointing out what that is and why you would want to use this (and not something else).
all the rest would then go into the docs and the docs will be pointed to from the readme and from the project homepage / from github.
The bullet points in the "supported" and "not supported" sections are not complete sentences and should therefore start with a lower case character and not end with a period. The "Not currently supported" section heading has also been improved to read better.
…ADME While py-esp32-ulp could be used on a PC to generate the ULP binary, which in turn could be uploaded to an ESP32, this workflow can also be realised using binutils-esp32ulp on the PC. py-esp32-ulp makes it easy to work directly on the ESP32 to assemble code for the ULP. However, the ESP32 is rather slow at doing this, which becomes relevant in battery-powered applications, where every second of sleep time is valuable. The documentation therefore no longer refers to the use-case of assembling ULP code on a PC, and adds a short reference to battery- powered applications as one reason, why one might want to split the assembly and load phase and store the ULP binary into a file.
I have now pushed 2 commits that address the comments given so far in the single README. I will now see how I can split out the detail into docs and keep the README only as a short intro (what, why and a quick start). |
The README now only covers the most important - mainly what this project is and how to get started. It then references the documentation for more detail. The documentation in turn contains more detail.
Mostly nicer/better phrasing. Add a Table of Contents to the documentation. Also change the Advanced Usage description from implying that "real-world" applications should use this approach to instead suggesting that this might be useful for "some" applications.
I have now trimmed down the README file and moved most of the detail to a documentation page. The "what is it useful for" and "features" list are now essentially duplicated between the README and the documentation file, but I the documentation page felt a bit incomplete without this, and referring (linking) back to the README for those felt a bit weird, given that the user likely came from the README. Let me know how you feel about it now. On the one hand I feel I was quite aggressive with cutting down the README, but on the other hand it seems to introduce the most important parts to the reader. I also wondered, if the "too long" problem now simply moved to the documentation page, but, since it's for readers that want more detail, I felt it was ok, and didn't try to break the documentation down further. (Was quite busy the last few weeks, so I couldn't move this along faster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some structure feedback.
maybe have a look at borgbackup/borg repo - I tried to have a "deduplicated README + docs" there.
https://raw.githubusercontent.com/borgbackup/borg/master/docs/index.rst
I have considered going down that route, but Github doesn't render this, and we currently have nothing to render documentation nor a place to host the documentation. I guess readthedocs.org can be one place to host them, but I tried to avoid adding even more work to get version 1.0 out the door and therefore aimed to make the rst file readable on Github. (Then again, maybe readthedocs does the heavy lifting and it's not much work?) Do you feel we should add documentation publishing to e.g. readthedocs.org? |
Hmm, ok, so I guess we should just have something for now that renders, but that is not duplicated. If that means that people are expected to have already read the README when reading the docs, then that's it for now. |
Also move badges to the top of README where they can be stripped away more easily when generating the project description for PyPI.
This is to avoid broken links in the PyPI project description (that occur because we avoid hardcoding the Github repository URLs in the README).
I have now de-duplicated the README and docs a bit more (the quick start is still in the README). I also addressed the broken links issue on the PyPI project description, by simply stripping away links (leaving the link names in "literal text" format) and by removing the badges as you suggested, when building the distribution package. This is what it looks like on Test PyPI now: https://test.pypi.org/project/micropython-py-esp32-ulp/. The code example also renders quite ok on the project description page. Happy to get more feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. If there is no other feedback, this can be merged.
@mattytrentini do you see any opportunity for making the README/documentation even better? Otherwise this can be merged as is. We can then discuss the next steps for finalising 1.0 in issue #49 |
Actually, let's merge this PR. If there are more comments/improvements, I can always create another PR. |
Thanks! |
Sorry folks, busy week - but I'm glad you merged this, it's a solid improvement! I'll try and provide further feedback over the weekend. |
This PR aims to improve the README, as suggested in #49, to make it easier to read and understand how to get started with py-esp32-ulp.
The main change to the README might be easier to look at by looking at the file rather than the patch.
The README has gotten much longer, and I did start wondering, whether it to shorten it and to move the rest into "Documentation". But, given that the most important content is at the top, I decided to keep it as is for now.
NOTE: For now we should not merge this, as I noticed the relative links used in the README don't work on PyPI (see https://test.pypi.org/project/micropython-py-esp32-ulp/). I will still try to find a solution, but for now I thought I could already get feedback on the README content/structure/friendliness/etc.